Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update public ip table with vuetify server side table #3946

Open
wants to merge 10 commits into
base: development
Choose a base branch
from
Open

Update public ip table with vuetify server side table #3946

wants to merge 10 commits into from

Conversation

samaradel
Copy link
Contributor

@samaradel samaradel commented Feb 26, 2025 •
edited
Loading

Description

Update the public IP table with Vuetify server side table

Related Issues

Tested Scenarios

Documentation PR

For UI changes, Please provide the Documentation PR on info_grid

To consider

Preliminary Checks:

  • Preliminary Checks
    • Does it completely address the issue linked?
    • What about edge cases?
    • Does it meet the specified acceptance criteria?
    • Are there any unintended side effects?
    • Does the PR adhere to the team's coding conventions, style guides, and best practices?
    • Does it integrate well with existing features?
    • Does it impact the overall performance of the application?
    • Are there any bottlenecks or slowdowns?
    • Has it been optimized for efficiency?
    • Has it been adequately tested with unit, integration, and end-to-end tests?
    • Are there any known defects or issues?
    • Is the code well-documented?
    • Are changes to documentation reflected in the code?

UI Checks:

  • UI Checks
    • If a UI design is provided/ does it follow it?
    • Does every button work?
    • Is the data displayed logical? Is it what you expected?
    • Does every validation work?
    • Does this interface feel intuitive?
    • What about slow network? Offline?
    • What if the gridproxy/graphql/chain is failing?
    • What would a first time user have a hard time navigating here?

Code Quality Checks:

  • Code Quality Checks
    • Code formatted/linted? Are there unused imports? .. etc
    • Is there redundant/repeated code?
    • Are there conditionals that are always true or always false?
    • Can we write this more concisely?
    • Can we reuse this code? If yes, where?
    • Will the changes be easy to maintain and update in the future?
    • Can this code become too complex to understand for other devs?
    • Can this code cause future integration problems?

Testing Checklist

  • Does it Meet the specified acceptance criteria.
  • Test if changes can affect any other functionality.
  • Tested with unit, integration, and end-to-end tests.
  • Tested the un-happy path and rollback scenarios.
  • Documentation updated to meet the latest changes.
  • Check automated tests working successfully with the changes.
  • Can be covered by automated tests.

General Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstring
  • Screenshots/Video attached (needed for UI changes)

@samaradel samaradel marked this pull request as ready for review March 2, 2025 20:48
0oM4R
0oM4R reviewed Mar 3, 2025
View reviewed changes
packages/playground/src/dashboard/components/public_ips_table.vue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
after changing the counts to 5 it is not changed and stay at 10

0oM4R
0oM4R requested changes Mar 3, 2025
View reviewed changes
packages/playground/src/dashboard/components/public_ips_table.vue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the. farm contains 17 ip and I can list only 10
image

packages/playground/src/dashboard/components/public_ips_table.vue Outdated
const farm = await gridStore.grid.farms.getFarmByID({ id });
publicIps.value = farm.publicIps as unknown as PublicIp[];
const { data, count } = await gridProxyClient.publicIps.list({
retCount: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure not to return count on each request. If I click on the next page, there is no need to return the count, this flag is very heavy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after passing the page and size it's lighter, please check

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check this issue, #1691; we should have the ret count only on the first request or whenever we need to get the count only

@samaradel samaradel marked this pull request as draft March 4, 2025 11:55
@samaradel samaradel marked this pull request as ready for review March 5, 2025 12:13
@samaradel samaradel requested a review from 0oM4R March 5, 2025 12:13
@0oM4R
Copy link
Contributor

0oM4R commented Mar 6, 2025

#3958 (comment) please check this refresh after adding IP is working fine on development

@samaradel samaradel marked this pull request as draft March 9, 2025 10:26
@samaradel samaradel marked this pull request as ready for review March 9, 2025 11:03
@samaradel
Copy link
Contributor Author

@0oM4R still not working

0oM4R
0oM4R reviewed Mar 10, 2025
View reviewed changes
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing the page is not working

Screen.Recording.2025-03-10.at.5.43.19.PM.mov

also refresh the table after add and delete IP is still not working, while it working fine on
https://dashboard.dev.grid.tf/#/farms/your-farms/

@samaradel samaradel requested a review from 0oM4R March 11, 2025 10:30
0oM4R
0oM4R reviewed Mar 11, 2025
View reviewed changes
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pagination is still not working properly, navigating between tabs is not updating the table data

Screen.Recording.2025-03-11.at.1.23.22.PM.mov

@samaradel samaradel marked this pull request as draft March 12, 2025 11:29
@samaradel samaradel requested a review from 0oM4R March 12, 2025 12:09
@samaradel samaradel marked this pull request as ready for review March 12, 2025 12:09
0oM4R
0oM4R reviewed Mar 12, 2025
View reviewed changes
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

same mentioned farm i got that error and also for 4292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0oM4R 0oM4R 0oM4R requested changes

@AhmedHanafy725 AhmedHanafy725 Awaiting requested review from AhmedHanafy725 AhmedHanafy725 is a code owner

@zaelgohary zaelgohary Awaiting requested review from zaelgohary zaelgohary is a code owner

@maayarosama maayarosama Awaiting requested review from maayarosama

@MohamedElmdary MohamedElmdary Awaiting requested review from MohamedElmdary MohamedElmdary is a code owner

@Mahmoud-Emad Mahmoud-Emad Awaiting requested review from Mahmoud-Emad Mahmoud-Emad is a code owner

@amiraabouhadid amiraabouhadid Awaiting requested review from amiraabouhadid amiraabouhadid is a code owner

@AlaaElattar AlaaElattar Awaiting requested review from AlaaElattar AlaaElattar is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@samaradel @0oM4R